Provide a unified (lsp-interface INTERFACE ...) pcase form #4559
Conversation
chrisbouchard
force-pushed
the
pcase-lsp-interface
branch
2 times, most recently
from
d308b1b
to
51c60f0
Compare
Is that Emacs 31.1 ? :D |
Could someone also report that slowdown to Emacs? |
Whoops, I think just 31. Not sure where the “1.” snuck in, but I removed it. Thanks 👍
Someone definitely could! I'm not using Emacs 31 yet, so I can't speak to that issue personally. I'm mostly interested in cleaning up pcase's docstring for readability. That said, depending what changes in Emacs 31 are triggering the slowdown, it might not actually be a bug. E.g., pcase's dynamic docstring function might have some new O(n²) feature that reasonably expects to handle a couple dozen pcase forms, not several hundred. |
chrisbouchard
force-pushed
the
pcase-lsp-interface
branch
2 times, most recently
from
ee7228d
to
5cf0a2e
Compare
This commit provides a new unified pcase form (lsp-interface INTERFACE ...) to replace the old per-interface (INTERFACE ...) forms -- the latter are now deprecated. (Unfortunately, I don't think there's a way to mark a pcase form as obsolete.) I've turned the existing pcase-defmacro definition into a helper function. The new pcase form delegates to that helper function, and the old pcase forms now delegate to the new form. This change addresses a few issues, which are detailed in emacs-lsp#4430. In short: * The existing forms aren't namespaced. * The lsp-mode package adds hundreds of forms, which each add several lines to pcase's generated docstring, adding up to over 1000 lines. * Starting in Emacs 31, the number of forms added by lsp-mode causes a noticeable slowdown when loading the interactive help for pcase.
I've tried to summarize the behavior of the existing implementation.
I used ripgrep to search for all occurrences of `pcase`, and then checked each to see if it was using a pattern that looked like an LSP interface name. It's very possible I missed something, but hopefully not.
The consensus in emacs-lsp#4430 is that we're ok with making this breaking change and communicating it in the CHANGELOG. I've replaced all uses in lsp-mode itself.
chrisbouchard
force-pushed
the
pcase-lsp-interface
branch
from
e9ffa48
to
49ff2a5
Compare
I'm an Org newb, so hopefully this is well-formatted.
chrisbouchard
force-pushed
the
pcase-lsp-interface
branch
from
49ff2a5
to
2ff5cdd
Compare
| * Replace the per-interface ~(INTERFACE ...)~ pcase forms with a single, | ||
| unified ~(lsp-interface INTERFACE ...)~ form. The per-interface forms are no | ||
| longer generated. *This is a breaking change.* (See #4430.) |
There was a problem hiding this comment.
I'm definitely open to suggestions on the CHANGELOG entry.
There was a problem hiding this comment.
Also, does the unreleased version stay 9.0.1, or should that go to 10.0.0 now?
|
There are two test suites failing, but they're for Emacs snapshots. Something's making the byte compiler unhappy. It looks like these are also failing in master. |
This has been reported upstream by @blahgeek (Yikai Zhao) https://debbugs.gnu.org/cgi/bugreport.cgi?bug=73766 and fixed in emacs-mirror/emacs@a815bec Along with emacs-lsp/lsp-mode#4559, this kludge should no longer be necessary
This pull request provides a new unified pcase form
(lsp-interface INTERFACE ...)to replace the old per-interface(INTERFACE ...)forms—the latter have been removed. This is a breaking change.I've turned the old
pcase-defmacros generated bylsp-interfaceinto helper functions, which the new pcase form delegates to.This change addresses a few issues, which are detailed in #4430. In short:
Fixes #4430